Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update Pants' own Python to 3.11 #21528

Merged
merged 2 commits into from
Nov 17, 2024
Merged

Conversation

cburroughs
Copy link
Contributor

@cburroughs cburroughs commented Oct 15, 2024

Building on the work in pantsbuild/scie-pants#351 this brings the version of Python used by Pants from 3.9 to 3.11. Why 3.11 and not 3.12 or 3.13? Because that is what we already released on the scie-pants side and two release forward is still a big benefit.

NOTE: I'd like to hold off on stomping out all deprecation warnings and anything else that "depends" on 3.11until one dev release since the release process part is what I'm most nervous about.

@cburroughs cburroughs force-pushed the csb/py11-bump branch 6 times, most recently from ca4fae3 to d4fa8dd Compare October 29, 2024 20:55
@cburroughs cburroughs changed the title python3.11 draft update Pants' own Python to 3.11 Oct 29, 2024
@cburroughs cburroughs force-pushed the csb/py11-bump branch 2 times, most recently from 1b4c9c4 to 64b26f0 Compare October 30, 2024 16:41
Copy link
Contributor Author

@cburroughs cburroughs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should add Thanks to:

@@ -18,6 +18,8 @@ extend-ignore:
NIC102,
# Unnecessary dict call - rewrite as a literal
C408,
# Temporarily exclude during Python upgrade
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self post merge.

@@ -63,7 +63,7 @@ class Target:
target_type: str = strawberry.field(
description="The target type, such as `python_sources` or `pex_binary` etc."
)
fields: JSONScalar = strawberry.field(
fields: JSONScalar = strawberry.field( # type: ignore[valid-type]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this clearly 'should work' given strawberry-graphql/strawberry#1205

but there are various similar looking errors at strawberry-graphql/strawberry#3579 Given the low current usage of the explorer I went with the ignore.

@@ -8,7 +8,7 @@
import os
import re
from dataclasses import dataclass, field
from enum import Enum
from enum import Enum, ReprEnum
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -22,6 +22,8 @@
PY_37 = "3.7"
PY_38 = "3.8"
PY_39 = "3.9"
PY_310 = "3.10"
PY_311 = "3.11"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added for completeness. I'm suspicious that some of these existing annotations around Python 3.9 semantically should be "the version of Python used by Pants" but have not pulled harder on that thread.

@cburroughs cburroughs marked this pull request as ready for review October 30, 2024 17:23
Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome, thank you! The code looks good, but want to nail down the deprecation/phasing details.

MINIMUM_SCIE_PANTS_VERSION = Version("0.10.0")
# First version with Python 3.11 support:
# https://github.com/pantsbuild/scie-pants/releases/tag/v0.12.0
MINIMUM_SCIE_PANTS_VERSION = Version("0.12.0")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we done any deprecations in advance? I'm thinking it's probably best if someone who's, say, got scie-pants 0.11.0 installed doesn't first learn about this by upgrading to 2.25.x and finding it fails hard/immediately.

Maybe we could sneak a warning about <0.12.0 into 2.24? (Either before branching or cherrypicked soon after)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We internally use an explicit version of scie-pants in ci/devcontainers, but I suspect that most people just use the bootstrapping script and end up with the latest and only update if forced to by this check or some issue they run into. 0.12.0 has been out since May so I think anyone is likely to have setup 0.11 and Pant 2.2{4,5} for the first time unless they were following some intentional internal procedure.

My fear we "per-deprectaiton" might be too clever by half and if we end up needing to put out a 0.12.x for some bug in this thing we are doing for the first time (Python upgrade) are are now forcing multiple upgrades.

@sureshjoshi had a PoC of using the new Pex-makes-the-scies support so that the Pants repo could control the python-build-standalone version and we would have less of this dance.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit groggy on this at the moment (and am not home to check), but one thing about deprecations... Which Python interpreter do in-repo plugins use? I'm struggling to recall if Pants runs them using it's own Python, or whether the user needs to have a system Python targeting 3.9.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which Python interpreter do in-repo plugins use?

I believe the answer is... both!

My understanding is that it "runs" them with with the Pants-Python, but if you have them setup as targets with BUILD files the linting/testing/etc happens with a 'regular' Python. (Thus the callout in the release notes plugin section.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My fear we "per-deprectaiton" might be too clever by half and if we end up needing to put out a 0.12.x for some bug in this thing we are doing for the first time (Python upgrade) are are now forcing multiple upgrades.

Yeah, that's a reasonable concern, but it seems unfortunate to give up on providing a user prompt just because we might get it wrong.

Thinking out loud, if we land this and put a pre-deprecation into 2.24 soon, we've got until 2.24.0 stable is released (i.e. at least a few weeks) to nail down any additional scie-pants changes and cherry-pick version bumps back to 2.24.x.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a heads up for plugin authors in the 2.23 blog post seams a reasonable thing we can do at the least.

I'm not opposed to some sort of warn-but-not-too-often in 2.24, I'm just not sure exactly what that looks like. Best effort to do something by 2.24.0 as we learn more in main sounds okay to me.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #21603

docs/notes/2.24.x.md Outdated Show resolved Hide resolved
Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this!

MINIMUM_SCIE_PANTS_VERSION = Version("0.10.0")
# First version with Python 3.11 support:
# https://github.com/pantsbuild/scie-pants/releases/tag/v0.12.0
MINIMUM_SCIE_PANTS_VERSION = Version("0.12.0")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #21603

@cburroughs
Copy link
Contributor Author

image
:-(

Created the minor followup issues discussed above. Intending to land this as soon as I figure out a path around the build breakage.

cburroughs added a commit that referenced this pull request Nov 16, 2024
While trying to land #21528 to was observed that the wheel building jobs
were failing during git checkout with wonderful errors like
`/__e/node20/bin/node: /lib64/libm.so.6: version `GLIBC_2.27' not found
(required by /__e/node20/bin/node)`. This appears to be the same
symptoms as actions/checkout#1809 which
pointed the the deprecation notice at
<https://github.blog/changelog/2024-03-07-github-actions-all-actions-will-run-on-node20-instead-of-node16-by-default/>.
If this all sounds familiar that is because we *mostly* cleaned this up
in #21133, but kept a fewer uses of the older actions for
`manylinux2014` compatibility. However, from the notice:

"To opt out of this and continue using Node16 while it is still
available in the runner, you can choose to set
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true
as an ‘env’ in their workflow or as an environment variable on your
runner machine. This will *only work until we upgrade the runner
removing Node16 later in the spring*. (emphasis added)"

From the wheel job failures during #21528 and my attempts to fiddle with
all of `ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION`,
`ACTIONS_RUNNER_FORCED_INTERNAL_NODE_VERSION`,
`ACTIONS_RUNNER_FORCE_ACTIONS_NODE_VERSION` I think the removal time
promised for the Spring has finally come, but I'm not that familiar with
GitHub Actions and could be missing something.

As a consequence the version of both `actions/upload-artifacts` and
`actions/checkout` is bumped to `v4`. To make this testable now and in
the future, wheels are now build on ci config changes.

Why is bumping to `manylinux_2_28` (the next oldest manylinux) not a big
deal? The "2.28" refers to glibc
[2.28](https://sourceware.org/legacy-ml/libc-alpha/2018-08/msg00003.html)
in 2018. That is older than Debian stable and if you need to use
software that old you are presumably paying someone for an enterprise
distribution.

NOTE: I think we should bump the manylinux version in `main` regardless,
but if my read if the situation is correct we may need to backport this
any active release line. Without this or a more complex change we would
also be unable to release after December 5th per #21616.

ref #21195 #21616
Building on the work in
pantsbuild/scie-pants#351 this brings the
version of Python used by Pants from 3.9 to 3.11.  Why 3.11 and
not 3.12 or 3.13?  Because that is what we already released on the
scie-pants side and two release forward is still a big benefit.

NOTE: I'd like to hold off on stomping out all deprecation warnings
until one `dev` release since the release process part is what I'm
most nervous about.
@cburroughs cburroughs merged commit 1258080 into pantsbuild:main Nov 17, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants